Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GitHubAPI fakes verification tests #4727

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

mikerkelly
Copy link
Contributor

Fix some issues in verification tests due to #4717.

Verification tests aren't run on branch/merge CI as the ones that hit the remote APIs are costly. I must have overlooked some issues locally (coverage) and/or introduced the error manually between locally testing and committing in #4717.

Function signatures on `FakeGitHubAPIWithErrors` ought to match `GitHubAPI`.
All the tests in these modules use `disable_db` so mark it at module level
rather than per class/test for clarity and brevity.
@mikerkelly mikerkelly force-pushed the mikerkelly/fix/github-fakes-verification branch from 75d13e9 to 18dc40a Compare November 8, 2024 11:44
Without this the tests in this module won't run when verification-tests.yml
workflow executes with "pytest -m verification".

See `pytest` docs [Specifying which tests to run](https://docs.pytest.org/en/stable/how-to/usage.html#specifying-which-tests-to-run)
and [Working with custom markers](https://docs.pytest.org/en/7.1.x/example/markers.html).
These classes are never instantiated, only compared, so the `coverage` tool
highlights them as not being covered by testing unless we mark them up as such.
@mikerkelly mikerkelly force-pushed the mikerkelly/fix/github-fakes-verification branch from f6c550c to 4ba08ff Compare November 8, 2024 12:31
@mikerkelly mikerkelly force-pushed the mikerkelly/fix/github-fakes-verification branch from 4ba08ff to e7311bb Compare November 8, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant